Skip to content

refactor(BA-5711): rename SessionData user_uuid to owner_id#11045

Closed
jopemachine wants to merge 6 commits into
refactor/BA-5650-B-user-permission-owner-idfrom
refactor/BA-5650-C-session-data-owner-id
Closed

refactor(BA-5711): rename SessionData user_uuid to owner_id#11045
jopemachine wants to merge 6 commits into
refactor/BA-5650-B-user-permission-owner-idfrom
refactor/BA-5650-C-session-data-owner-id

Conversation

@jopemachine
Copy link
Copy Markdown
Member

@jopemachine jopemachine commented Apr 14, 2026

📚 Stack

Merge from top to bottom. Intermediate slices may not build standalone; the final refactor tip is #11051, and #11040 is the schema-drop follow-up on top of it.

Summary

Data-layer rename of SessionData/SessionMetadata.user_uuidowner_id; drop redundant access_key snapshot fields. SessionRow.to_dataclass/from_dataclass/to_session_info/from_session_info adapters updated. ComputeSessionNode.from_dataclass becomes async, resolves main_access_key via UserRepository. models/resource_usage.py sources user_id from SessionRow.user_uuid. _build_session_fetch_query/_match_sessions_by_* helpers rename filter parameter.

Resolves BA-5711. Part of epic BA-5650.


📚 Documentation preview 📚: https://sorna--11045.org.readthedocs.build/en/11045/


📚 Documentation preview 📚: https://sorna-ko--11045.org.readthedocs.build/ko/11045/


BA-5650 Series: Split Rationale

Overall goal: migrate the session owner identifier from access_key (keypair) to owner_id (user UUID), and drop the sessions.access_key / kernels.access_key columns.

Split criteria: layer + dependency order. Bottom-up (DB helpers → service → API) so the destructive column drop can land safely at the end.

Order Issue Layer Role
1 BA-5709 Foundation Add get_main_access_key_by_id and related resolver helpers (everything else depends on this)
2 BA-5710 RBAC / Permission Rename UserPermission.user_uuid → owner_id; add main_access_key field
3 BA-5711 Data Rename SessionData.user_uuid → owner_id; Row adapters; GQL node
4 BA-5712 Repository Collapse SessionRepository, SessionDBSource, creators signatures
5 BA-5713 Scheduler predicates / drf / options; scheduler repo; stream/events shims
6 BA-5714 Sokovan allocation / lifecycle / workload, launcher, controller, executor
7 BA-5715 Service Drop owner_id from 21 read/control Actions; resolve via current_user()
8 BA-5716 API (v1) Remove owner_access_key from REST v1 DTOs (breaking)
9 BA-5717 Tests / Cleanup Test suite updates; remaining ORM / gql_legacy touch-ups
10 BA-5653 Schema Alembic migration — rename/drop columns (destructive, must land last)

Why this split

  • Dependencies: the BA-5709 helpers are required before 3–7 can recover access_key from owner_id. BA-5653 (destructive) only runs once every reader has migrated off the dropped column.
  • Review size: a single-shot change would touch hundreds of files. Slicing by layer keeps each PR focused on one concern.
  • Rollback safety: steps 1–7 are no-op on external behavior, step 8 is the API breaking change, step 10 is the schema drop. Each step is independently revertible.

@github-actions github-actions Bot added size:L 100~500 LoC comp:manager Related to Manager component labels Apr 14, 2026
@github-actions github-actions Bot added the area:docs Documentations label Apr 14, 2026
@jopemachine jopemachine changed the title refactor(BA-5650-C): rename SessionData user_uuid to owner_id refactor(BA-5711): rename SessionData user_uuid to owner_id Apr 14, 2026
@jopemachine jopemachine marked this pull request as draft April 14, 2026 07:30
@jopemachine jopemachine force-pushed the refactor/BA-5650-B-user-permission-owner-id branch from 6d34e21 to d88a14b Compare April 14, 2026 07:39
@jopemachine jopemachine force-pushed the refactor/BA-5650-C-session-data-owner-id branch from 0b27565 to caa8001 Compare April 14, 2026 07:39
@jopemachine jopemachine requested a review from Copilot April 14, 2026 07:46
Comment thread changes/11045.misc.md Outdated
Comment thread src/ai/backend/manager/api/gql_legacy/session.py Outdated
@jopemachine jopemachine force-pushed the refactor/BA-5650-C-session-data-owner-id branch from caa8001 to 7bac45f Compare April 14, 2026 07:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors session data ownership identifiers by renaming SessionData.user_uuid / SessionMetadata.user_uuid to owner_id and removing redundant access-key snapshot fields, while updating adapters and query helpers to use owner_id and resolve main_access_key from the owning user.

Changes:

  • Rename session dataclass fields to owner_id and drop access_key snapshot fields.
  • Update SessionRow adapters (to_dataclass/from_dataclass, to_session_info/from_session_info) and session fetch/match helpers to scope by owner_id.
  • Update legacy GraphQL session node building to resolve main_access_key (async fallback) and update resource-usage computation to source access key from the user relationship.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/ai/backend/manager/models/session/row.py Refactors session lookup helpers to owner_id, updates dataclass/session-info adapters, removes keypair relationship, and renames a session-name uniqueness index.
src/ai/backend/manager/models/resource_usage.py Switches resource-usage access-key sourcing to user.main_access_key and removes SessionRow.access_key from selected columns.
src/ai/backend/manager/data/session/types.py Renames user_uuidowner_id and removes access-key snapshot fields from session dataclasses.
src/ai/backend/manager/api/gql_legacy/session.py Makes ComputeSessionNode.from_dataclass async and resolves owner main_access_key via eager owner data or UserRepository.
changes/11045.misc.md Adds a changelog entry describing the rename and access-key snapshot removal.
Comments suppressed due to low confidence (1)

src/ai/backend/manager/models/session/row.py:863

  • Renaming the SQLAlchemy index to ix_sessions_unique_name_per_owner_nonterminal without a matching Alembic migration will make schema comparisons/autogenerate treat this as a drop/create (the existing migration creates ix_sessions_unique_name_per_user_nonterminal; see models/alembic/versions/a4289ef5f0cd_allow_duplicated_session_names_across_.py). Either keep the existing index name for compatibility or add a migration that renames/drops+creates the index with the new name.
        sa.Index(
            "ix_sessions_unique_name_per_owner_nonterminal",
            "name",
            "user_uuid",
            unique=True,
            postgresql_where=sa.text("status NOT IN ('ERROR', 'TERMINATED', 'CANCELLED')"),
        ),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ai/backend/manager/models/session/row.py
Comment thread src/ai/backend/manager/models/session/row.py
Comment thread src/ai/backend/manager/models/session/row.py
Comment thread src/ai/backend/manager/models/resource_usage.py
Comment thread src/ai/backend/manager/models/resource_usage.py Outdated
Comment thread src/ai/backend/manager/models/resource_usage.py Outdated
Comment thread src/ai/backend/manager/models/session/row.py
Comment thread src/ai/backend/manager/api/gql_legacy/session.py Outdated
Comment thread src/ai/backend/manager/models/session/row.py
@jopemachine jopemachine force-pushed the refactor/BA-5650-B-user-permission-owner-id branch from 5ea1195 to cfdf874 Compare April 14, 2026 07:58
Data-layer rename of SessionData / SessionMetadata user_uuid to
owner_id, plus the matching SessionRow adapters
(to_dataclass/from_dataclass/to_session_info/from_session_info).
ComputeSessionNode.from_dataclass becomes async and resolves
main_access_key from the owner via UserRepository when owner is not
eagerly loaded. models/resource_usage.py sources user_id from
SessionRow.user_uuid through the session relationship.

SessionRow._build_session_fetch_query / _match_sessions_by_* also
rename the filter parameter to owner_id; repository and db_source
callers are updated in the next slice so intermediate builds may
fail in isolation.
@jopemachine jopemachine force-pushed the refactor/BA-5650-C-session-data-owner-id branch from 7bac45f to d151254 Compare April 14, 2026 07:59
- api/gql_legacy/session.py: make ComputeSessionNode.from_dataclass
  synchronous again; main_access_key is now passed in by the caller
  (typically via UserRepository.get_main_access_key_by_id). Callers
  in ModifyComputeSession.mutate / CheckAndTransitStatus.mutate
  resolve it ahead of time and pass through. Avoids a hidden per-node
  DB round-trip and lets the caller batch the lookup.
- models/keypair/row.py: drop the ``sessions`` relationship (and the
  join-condition helper + TYPE_CHECKING import) that was
  back-populating ``SessionRow.access_key_row`` — that relationship
  no longer exists on SessionRow after this slice.
- models/resource_usage.py: rewrite stale TODO comments that said
  ``SessionRow.access_key has been removed`` — the column stays in
  this slice; it is deprecated and dropped in a later one. Also add
  ``UserRow.main_access_key`` to the ``load_only`` list so the field
  is hydrated when ``kern.session.user.main_access_key`` is read.
- models/session/row.py: update ``match_sessions`` and ``get_session``
  docstrings to describe ``owner_id`` instead of ``access_key``.

Also drop the stale misc news fragment (slice is ``skip:changelog``).
@jopemachine jopemachine added the skip:changelog Make the action workflow to skip towncrier check label Apr 14, 2026
@jopemachine jopemachine removed the skip:changelog Make the action workflow to skip towncrier check label Apr 14, 2026
jopemachine and others added 2 commits April 14, 2026 17:26
After renaming SessionData/SessionMetadata fields and the
SessionRow.get_session / match_sessions helpers, in-repo callers that
used the old keyword argument (``access_key=``) or positional
signature need the matching updates so slice C builds standalone:

- manager/registry.py, repositories/stream/db_source/db_source.py,
  repositories/session/dependency_graph.py: switch to the new
  ``owner_id`` keyword argument on SessionRow.get_session /
  match_sessions.
- models/endpoint/row.py: drop the ``target_access_key`` argument on
  SessionRow.delegate_ownership (its signature was trimmed by slice B).
- tests/unit/manager/sokovan/scheduler/handlers/conftest.py: move
  SessionMetadata fixture fields from ``user_uuid``/``access_key`` to
  ``owner_id``; downstream access via ``metadata.owner_id`` and the
  rows now read ``main_access_key`` via the user record.
Slice C renames SessionData fields but left several downstream call
sites broken in isolation (intentional per the original commit
message). Fixing those so each PR in the stack passes its own CI:

- ``SessionRow.match_sessions`` / ``get_session`` accept an optional
  ``owner_access_key`` keyword that resolves to the owner via the
  ``users.main_access_key`` sub-query — lets existing callers keep
  their AccessKey-based filter while the column rename completes.
- ``find_dependency_sessions`` now takes ``owner: UUID | AccessKey``
  and dispatches to the matching filter.
- session/db_source call sites updated: positional AccessKey → keyword
  ``owner_access_key=…``.
- Scheduler handlers test conftest temporarily uses ``access_key`` /
  ``user_uuid`` field names until the rename lands in slice F.
- ``SessionAdapter._session_data_to_node`` reads ``data.owner_id`` and
  drops the obsolete ``data.access_key`` lookup.
- ``ModelServingRepository.get_session_by_id`` drops the obsolete
  positional arg.
- ``UserDBSource.delegate_endpoint_ownership`` discards the
  ``target_main_access_key`` arg (the ORM helper no longer uses it).
- ``AgentRegistry`` stops threading ``user_repository`` into the
  ``SessionLifecycleManager`` constructor (the parameter is added in
  slice G).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jopemachine
Copy link
Copy Markdown
Member Author

Consolidated into the merged PR stack (A+B+C+D, E+F, G+H, I)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentations comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants